-
Notifications
You must be signed in to change notification settings - Fork 897
SSH API Implementation #852
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -0,0 +1 @@ | |||
True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be set to false for production purposes. Until we decide to embed libssh2 into supplied binaries.
/cc @nulltoken |
buuump? |
@Therzok Sorry for the delay. Below, my thoughts. If we're going the libssh2 road, maybe should do it completely and
However, I'm not a ssh expert, and I'd rather have some better 👀 on this as well. (@carlosmn, @ethomson ?) |
If you're going to include libssh2 by default, it needs to be easily disabled, as this is only a workaround for Windows users. Everyone else has an easy way to install libssh2 and have libgit2 detect it on build. There is one upstream repo, so that bit shouldn't be an issue. I'm not sure if the lg2s repo should have the submodule itself, for the same reason that it's a Windows end-user convenience to include libssh2 in lg2. |
The way I did it locally is have libssh2 side by side with libgit2sharp. That's why I had conditional inclusion. I don't know, I thought this was the best option to support all platforms and libssh2 shipping combinations sanely. |
SSH is something that I definitely cannot ship. I very much want this to stay optional. |
I need comments on a few things:
|
@@ -33,7 +36,8 @@ $build_clar = 'OFF' | |||
if ($test.IsPresent) { $build_clar = 'ON' } | |||
$configuration = "RelWithDebInfo" | |||
if ($debug.IsPresent) { $configuration = "Debug" } | |||
|
|||
$embed_ssh = '' | |||
if ($ssh.IsPresent) { $embed_ssh = '-DEMBED_SSH_PATH="../../libssh2"' } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than an $ssh
switch, maybe accept this path value as the parameter (defaulted to $null
)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than an
$ssh
switch, maybe accept this path value as the parameter (defaulted to$null
)?
👍 as well
6824b67
to
283d369
Compare
So, to revisit this, I need the comments stated above. What I have until now: |
d3be444
to
dd72ce8
Compare
ab37cbb
to
e0a98ce
Compare
Should be good to go now. Tested with libssh2 on my side, and it works. |
We should probably enforce coverage on these protocols and probably setup bots to also test ssh features. Thoughts? |
We test the ssh integration in libgit2, you can take a look at the script we use there to grab the key and set up the ssh daemon so we can log in. |
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalCookie = UniqueId.UniqueIdentifier, MarshalTypeRef = typeof(StrictUtf8Marshaler))] string username, | ||
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalCookie = UniqueId.UniqueIdentifier, MarshalTypeRef = typeof(StrictUtf8Marshaler))] string publickey, | ||
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalCookie = UniqueId.UniqueIdentifier, MarshalTypeRef = typeof(StrictUtf8Marshaler))] string privatekey, | ||
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalCookie = UniqueId.UniqueIdentifier, MarshalTypeRef = typeof(StrictUtf8Marshaler))] string passphrase); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is most likely wrong, we need an ASCII string passed, from what I saw.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/cc @carlosmn
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see why we would want to restrict this to ASCII. UTF-8 is compatible with ASCII and what happens if I write my password in Spanish or German? I would need more letters than ASCII there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, dunno why sending the passphrase in the current state isn't succeeding authentication.
I thought maybe libssh2 or something didn't support unicode, I'll investigate
@nulltoken what's the magic needed to be done to get this PR to have remote tests? :D |
|
I'll get on rebasing this tomorrow on top of vNext. Wish me luck. |
IIRC @carlosmn did the hard work making this happen for libgit2 |
So, we don't need to test it libgit2sharp side? :P Else, I'll just grab libgit2's credentials and do some magic. |
The ssh tests are a bit of a hassle to set up, but you can copy the code from libgit2's Travis script. I would recommend having at least one test run there to make sure basic functionality doesn't get broken. |
So as a library consumer via nuget any docs or instructions updates on how I use SSH in LibGit2Sharp? |
old issue but here's how you can do it now,
Then pass that into the
|
Why is SSH support in a separate package "LibGit2Sharp-SSH" (v1.0.14) from the main NuGet package "LibGit2Sharp"? Is there a difference, other than one has SSH support and the other does not? I already noticed that a couple StatusOptions are gone: "IncludeUntracked" and "RecurseUntrackedDirs". Not a big deal, since those never made sense as options anyway. I'm also curious what the SshAgentCredentials class is for. It only has a "Username" property. When I try to use that, I just get an error saying "Failed to start SSH session: Unable to exchange encryption keys". Is that for having some agent running on the system that will automatically supply encryption keys? I currently have Putty/Pageant and Kleopatra running on my system. Update: It seems there's a newer package called "LibGit2Sharp-SSH-updated-libssh2" (v1.0.25) (and dependent package "LibGit2Sharp-SSH.NativeBinaries-libssh2-1.8.1.0" (v1.0.15)). Is that what I'm supposed to use instead of "LibGit2Sharp-SSH"? Is that why I'm seeing the error previously mentioned? Of course now, using the newer library, I get the error "[DllNotFoundException: Unable to load DLL 'git2-ssh-baa87df': The specified module could not be found. (Exception from HRESULT: 0x8007007E)] LibGit2Sharp.Core.NativeMethods.git_libgit2_init() +0". Update 2: It seems I have to manually copy the "git2-ssh-baa87df.dll" file from the package to the bin directory, and have to use the x86 format to avoid a "bad image" error, presumably because running this web app locally uses 32-bit IIS. However, even using the x86 version, I get the error "DllNotFoundException: Unable to load DLL 'git2-ssh-baa87df': The specified module could not be found. (Exception from HRESULT: 0x8007007E)." Update 3: Apparently, the package throws this "fixbuild.txt" file into the project, which I guess I'm supposed to take a hint from that the build needs fixed. Searching for the source of this file, I find another file alongside it with a '.props' extension, which appears to include visual studio project ItemGroup entries that need manually added to my project file. After adding them, the paths were wrong, so I had to update the paths to point relative to the package folder. Where are all the instructions for this stuff? Futhermore, the location is copies them "lib/win32/x86" and "lib/win32/x64" are not sufficient. The right files need copied from there directly into the bin directory to avoid the DLL not found error. Update 4: After getting all the files seemingly where they need to be, including the property x86 files in the bin directory, I'm back to getting the original error of "Failed to start SSH session: Unable to exchange encryption keys". |
Some 3rd party has created this If you need SSH support, I would encourage you to build your own libgit2 that uses libssh2. This is not ideal but basically the only mechanism that will work today. |
This isn't yet rebased, nor is it mergeable, it's just up for review and comments.